fix bug from issue #460 #464
Conversation
|
@jquense ? |
|
Reviewing your PR now. 👀 |
There was a problem hiding this comment.
Could you give more details about what your tests are proving? By "at once" do you mean "immediately"? How is time measured here?
I was thinking that actually 0 does make sense as a default timeout value for enter and exit. I think this would be the most logical step forward and wouldn't cause warnings for existing code. What do you think?
If you agree, would you mind reverting the change for prop types and instead of current tests add simpler tests where by measuring time you ensure that:
- in the absence of
enter, the status upon entering immediately becomesentered - in the absence of
exit, the status upon exiting immediately becomesexited
I don't think we have to test the unmountOnExit prop for this, we just want to ensure the default values.
|
Concerning prop types, you can take this opportunity to actually fix them: PropTypes.oneOfType([
PropTypes.number,
PropTypes.shape({
enter: PropTypes.number,
exit: PropTypes.number,
appear: PropTypes.number,
})
]).isRequiredBy moving I'm honestly not sure what |
Co-Authored-By: dimensi <[email protected]>
yes, i don't know english, i use google translate for find a word. |
|
Awesome, thank you! 🏆 |
|
@silvenon thx for your review. Are you sure my tests right writed? |
|
@dimensi yep, they look good to me. I didn't even think to combine both functionalities in the same test. We might later split that into two tests because they are two different features. |
|
🎉 This PR is included in version 2.6.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
I found an #460 with a bug, since the bug is simple, I decided to correct the error.
The error was that we did not take into account the case when there is a node and there is no timeout.
I also set the enter and exit values in prop-types as reqiured.